#2250 dragonfruit #2360
#2250 dragonfruit #2360
Conversation
|
WalkthroughThe changes introduce the optional Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/core/test/taskExecutor.test.ts (1)
947-967: LGTM! Consider adding edge case coverage.The test case correctly validates that
concurrencyKeyis properly passed through to the task context. The implementation follows existing patterns and adequately tests the happy path.Consider adding a complementary test case to verify behavior when
concurrencyKeyisundefined:+ test("should handle undefined concurrencyKey in task context", async () => { + let capturedContext: any; + + const task = { + id: "test-task", + fns: { + run: async (payload: any, params: RunFnParams<any>) => { + capturedContext = params.ctx; + return { + output: "test-output", + }; + }, + }, + }; + + const result = await executeTask(task, { test: "data" }); + + expect(result.result.ok).toBe(true); + expect(capturedContext).toBeDefined(); + expect(capturedContext.run.concurrencyKey).toBeUndefined(); + });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/webapp/app/routes/resources.runs.$runParam.ts(1 hunks)apps/webapp/app/v3/marqs/sharedQueueConsumer.server.ts(2 hunks)apps/webapp/app/v3/services/createTaskRunAttempt.server.ts(1 hunks)packages/core/src/v3/schemas/common.ts(2 hunks)packages/core/test/taskExecutor.test.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Always prefer using isomorphic code like fetch, ReadableStream, etc. instead of Node.js specific code
For TypeScript, we usually use types over interfaces
Avoid enums
No default exports, use function declarations
Files:
apps/webapp/app/routes/resources.runs.$runParam.tsapps/webapp/app/v3/services/createTaskRunAttempt.server.tspackages/core/test/taskExecutor.test.tspackages/core/src/v3/schemas/common.tsapps/webapp/app/v3/marqs/sharedQueueConsumer.server.ts
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
We use zod a lot in packages/core and in the webapp
Files:
apps/webapp/app/routes/resources.runs.$runParam.tsapps/webapp/app/v3/services/createTaskRunAttempt.server.tspackages/core/test/taskExecutor.test.tspackages/core/src/v3/schemas/common.tsapps/webapp/app/v3/marqs/sharedQueueConsumer.server.ts
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: In the webapp, all environment variables must be accessed through theenvexport ofenv.server.ts, instead of directly accessingprocess.env.
When importing from@trigger.dev/corein the webapp, never import from the root@trigger.dev/corepath; always use one of the subpath exports as defined in the package's package.json.
Files:
apps/webapp/app/routes/resources.runs.$runParam.tsapps/webapp/app/v3/services/createTaskRunAttempt.server.tsapps/webapp/app/v3/marqs/sharedQueueConsumer.server.ts
**/*.test.{ts,tsx}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
Our tests are all vitest
Files:
packages/core/test/taskExecutor.test.ts
🧠 Learnings (14)
📓 Common learnings
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : When implementing schema tasks, use `schemaTask` from `trigger.dev/sdk/v3` and validate payloads as shown.
📚 Learning: in apps/webapp/app/services/runsrepository.server.ts, the in-memory status filtering after fetching ...
Learnt from: matt-aitken
PR: triggerdotdev/trigger.dev#2264
File: apps/webapp/app/services/runsRepository.server.ts:172-174
Timestamp: 2025-07-12T18:06:04.133Z
Learning: In apps/webapp/app/services/runsRepository.server.ts, the in-memory status filtering after fetching runs from Prisma is intentionally used as a workaround for ClickHouse data delays. This approach is acceptable because the result set is limited to a maximum of 100 runs due to pagination, making the performance impact negligible.
Applied to files:
apps/webapp/app/routes/resources.runs.$runParam.ts
📚 Learning: do not use or add new code to the legacy run engine; focus on using and migrating to run engine 2.0 ...
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-07-18T17:49:47.180Z
Learning: Do not use or add new code to the legacy run engine; focus on using and migrating to Run Engine 2.0 in `internal/run-engine`.
Applied to files:
apps/webapp/app/routes/resources.runs.$runParam.ts
📚 Learning: applies to **/trigger/**/*.{ts,tsx,js,jsx} : the `run` function contains your task logic in trigger....
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : The `run` function contains your task logic in Trigger.dev tasks.
Applied to files:
apps/webapp/app/routes/resources.runs.$runParam.tsapps/webapp/app/v3/services/createTaskRunAttempt.server.tspackages/core/test/taskExecutor.test.tspackages/core/src/v3/schemas/common.ts
📚 Learning: applies to **/trigger/**/*.{ts,tsx,js,jsx} : when using realtime features, use the `runs.subscribeto...
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : When using Realtime features, use the `runs.subscribeToRun`, `runs.subscribeToRunsWithTag`, and `runs.subscribeToBatch` APIs as shown.
Applied to files:
apps/webapp/app/routes/resources.runs.$runParam.ts
📚 Learning: applies to **/trigger/**/*.{ts,tsx,js,jsx} : when using metadata in tasks, use the `metadata` api as...
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : When using metadata in tasks, use the `metadata` API as shown, and only inside run functions or task lifecycle hooks.
Applied to files:
apps/webapp/app/v3/services/createTaskRunAttempt.server.tspackages/core/test/taskExecutor.test.ts
📚 Learning: applies to **/trigger/**/*.{ts,tsx,js,jsx} : when using retry, queue, machine, or maxduration option...
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : When using retry, queue, machine, or maxDuration options, configure them as shown in the examples for Trigger.dev tasks.
Applied to files:
apps/webapp/app/v3/services/createTaskRunAttempt.server.tspackages/core/test/taskExecutor.test.tsapps/webapp/app/v3/marqs/sharedQueueConsumer.server.ts
📚 Learning: in `packages/core/src/v3/errors.ts`, within the `taskrunerrorenhancer` function, `error.message` is ...
Learnt from: nicktrn
PR: triggerdotdev/trigger.dev#1418
File: packages/core/src/v3/errors.ts:364-371
Timestamp: 2024-10-18T15:41:52.352Z
Learning: In `packages/core/src/v3/errors.ts`, within the `taskRunErrorEnhancer` function, `error.message` is always defined, so it's safe to directly call `error.message.includes("SIGTERM")` without additional checks.
Applied to files:
packages/core/test/taskExecutor.test.tspackages/core/src/v3/schemas/common.tsapps/webapp/app/v3/marqs/sharedQueueConsumer.server.ts
📚 Learning: applies to **/trigger/**/*.{ts,tsx,js,jsx} : tasks must be exported, even subtasks in the same file....
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Tasks must be exported, even subtasks in the same file.
Applied to files:
packages/core/test/taskExecutor.test.ts
📚 Learning: applies to **/trigger/**/*.{ts,tsx,js,jsx} : if you are able to generate an example payload for a ta...
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : If you are able to generate an example payload for a task, do so.
Applied to files:
packages/core/test/taskExecutor.test.ts
📚 Learning: applies to **/trigger/**/*.{ts,tsx,js,jsx} : when implementing schema tasks, use `schematask` from `...
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : When implementing schema tasks, use `schemaTask` from `trigger.dev/sdk/v3` and validate payloads as shown.
Applied to files:
packages/core/src/v3/schemas/common.ts
📚 Learning: applies to **/trigger/**/*.{ts,tsx,js,jsx} : when implementing scheduled (cron) tasks, use `schedule...
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : When implementing scheduled (cron) tasks, use `schedules.task` from `trigger.dev/sdk/v3` and follow the shown patterns.
Applied to files:
packages/core/src/v3/schemas/common.tsapps/webapp/app/v3/marqs/sharedQueueConsumer.server.ts
📚 Learning: applies to **/trigger/**/*.{ts,tsx,js,jsx} : always generate trigger.dev tasks using the `task` func...
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : ALWAYS generate Trigger.dev tasks using the `task` function from `trigger.dev/sdk/v3` and export them as shown in the correct pattern.
Applied to files:
packages/core/src/v3/schemas/common.ts
📚 Learning: applies to **/trigger/**/*.{ts,tsx,js,jsx} : you must use `@trigger.dev/sdk/v3` when writing trigger...
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : You MUST use `trigger.dev/sdk/v3` when writing Trigger.dev tasks.
Applied to files:
packages/core/src/v3/schemas/common.ts
🔇 Additional comments (8)
apps/webapp/app/routes/resources.runs.$runParam.ts (1)
192-192: LGTM! Consistent implementation following existing patterns.The addition of
concurrencyKeyto the context follows the same pattern as other optional fields and correctly handles null values.apps/webapp/app/v3/services/createTaskRunAttempt.server.ts (1)
223-223: LGTM! Proper propagation of concurrencyKey to execution context.The addition correctly includes the concurrencyKey in the execution payload, maintaining consistency with the existing codebase patterns.
packages/core/src/v3/schemas/common.ts (2)
225-225: LGTM! Schema definition correctly adds optional concurrencyKey field.The addition of
concurrencyKeyas an optional string field to the TaskRun schema is appropriate and consistent with the feature requirements.
379-379: LGTM! Consistent schema update for V3TaskRun.The addition maintains consistency between TaskRun and V3TaskRun schemas, ensuring the concurrencyKey field is available in both versions.
apps/webapp/app/v3/marqs/sharedQueueConsumer.server.ts (2)
1642-1642: LGTM! Database selection includes concurrencyKey field.The addition ensures that the concurrencyKey is retrieved from the database when constructing execution payloads, which is essential for the feature to work end-to-end.
1726-1726: LGTM! Execution payload properly includes concurrencyKey.The addition correctly propagates the concurrencyKey from the database query into the execution payload, completing the end-to-end flow for this feature.
packages/core/test/taskExecutor.test.ts (2)
871-872: LGTM! Clean function signature extension.The optional
concurrencyKeyparameter is properly added while maintaining backward compatibility. The placement at the end of the parameter list follows good API design practices.
1930-1930: LGTM! Proper execution object assignment.The
concurrencyKeyis correctly assigned to theexecution.runobject, maintaining consistency with other run properties and aligning with the schema updates.
special pr for eric
Summary of Changes
I successfully implemented the feature to add
concurrencyKeyto the task run context. Here's a comprehensive overview of the changes made:1. Schema Updates (
packages/core/src/v3/schemas/common.ts)concurrencyKey: z.string().optional()to theV3TaskRunschema (line 378)concurrencyKey: z.string().optional()to theTaskRunschema (line 225) for backward compatibility2. Database Query Updates (
apps/webapp/app/v3/marqs/sharedQueueConsumer.server.ts)concurrencyKey: trueto theAttemptForExecutionGetPayload.select.taskRun.select(line 1642)concurrencyKey: taskRun.concurrencyKey ?? undefinedto the execution object creation (line 1726)3. Execution Context Updates (
apps/webapp/app/v3/services/createTaskRunAttempt.server.ts)concurrencyKey: taskRun.concurrencyKey ?? undefinedto the execution object creation (line 223)include, so no query changes were needed4. UI Context Updates (
apps/webapp/app/routes/resources.runs.$runParam.ts)concurrencyKey: run.concurrencyKey ?? undefinedto the context object creation (line 192)concurrencyKey: true(line 55)5. Test Coverage (
packages/core/test/taskExecutor.test.ts)concurrencyKeyis properly included in the task context (lines 947-967)executeTaskhelper function to accept an optionalconcurrencyKeyparameter (line 1872)concurrencyKeyfield (line 1930)How the Implementation Works
Triggering: When a task is triggered with a
concurrencyKey, it's stored in the databaseTaskRun.concurrencyKeyfield (this was already working)Database Retrieval: When creating task executions, the
concurrencyKeyis now selected from the database in all relevant queriesContext Building: The
concurrencyKeyis included in theV3TaskRunExecution.runobject when building the execution contextSchema Validation: The updated
V3TaskRunandTaskRunschemas now include theconcurrencyKeyfield, ensuring type safetyContext Parsing: The existing context parsing logic in the CLI workers (
TaskRunContext.parse(execution)) automatically handles the new field since it's part of the schemaValidation
The implementation follows the existing patterns in the codebase:
concurrencyKeyis already stored in theTaskRuntableV3TaskRunandTaskRunschemas updatedconcurrencyKeyconcurrencyKeyUsage Example
Now developers can access the
concurrencyKeyin their tasks:The implementation is now complete and ready for use! The
concurrencyKeywill be properly passed through from the trigger options to the task context, making it available to developers in their task implementations.